feat(extensions): support multiple active catalogs simultaneously#1720
feat(extensions): support multiple active catalogs simultaneously#1720
Conversation
- Add CatalogEntry dataclass to represent catalog entries - Add get_active_catalogs() reading SPECKIT_CATALOG_URL, project config, user config, or built-in default stack (org-approved + community) - Add _load_catalog_config() to parse .specify/extension-catalogs.yml - Add _validate_catalog_url() HTTPS validation helper - Add _fetch_single_catalog() with per-URL caching, backward-compat for DEFAULT_CATALOG_URL - Add _get_merged_extensions() that merges all catalogs (priority wins on conflict) - Update search() and get_extension_info() to use merged results annotated with _catalog_name and _install_allowed - Update clear_cache() to also remove per-URL hash cache files - Add extension_catalogs CLI command to list active catalogs - Add catalog add/remove sub-commands for .specify/extension-catalogs.yml - Update extension_add to enforce install_allowed=false policy - Update extension_search to show source catalog per result - Update extension_info to show source catalog with install_allowed status - Add 13 new tests covering catalog stack, merge conflict resolution, install_allowed enforcement, and catalog metadata Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
…port - RFC: replace FUTURE FEATURE section with full implementation docs, add catalog stack resolution order, config file examples, merge conflict resolution, and install_allowed behavior - EXTENSION-USER-GUIDE.md: add multi-catalog section with CLI examples for catalogs/catalog-add/catalog-remove, update catalog config docs - EXTENSION-API-REFERENCE.md: add CatalogEntry class docs, update ExtensionCatalog docs with new methods and result annotations, add catalog CLI commands (catalogs, catalog add, catalog remove) Also fix extension_catalogs command to correctly show "Using built-in default catalog stack" when config file exists but has empty catalogs Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
|
@copilot Address the following Downloading ruff (10.7MiB) F541 [*] f-string without any placeholders F541 [*] f-string without any placeholders Found 3 errors. |
Remove f-prefix from strings with no placeholders in catalog_remove and extension_search commands. Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR implements multi-catalog support for the Spec Kit extension system, replacing the single active catalog model with a composable "catalog stack." By default, both the org-approved catalog.json (installable) and catalog.community.json (discovery-only) are active simultaneously, enabling out-of-the-box community discoverability while preserving org curation controls.
Changes:
- Catalog stack engine: New
CatalogEntrydataclass,get_active_catalogs(),_load_catalog_config(),_fetch_single_catalog(), and_get_merged_extensions()methods inextensions.py— implementing the full resolution order (env var → project config → user config → built-in defaults) - CLI commands: Three new commands added to
__init__.py:specify extension catalogs,specify extension catalog add, andspecify extension catalog remove; plus updatedextension_add,extension_search, andextension_infoto surface_install_allowed/_catalog_namemetadata - Tests and docs: 13 new tests in
TestCatalogEntryandTestCatalogStack; comprehensive updates to RFC, User Guide, and API Reference
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/extensions.py |
Core multi-catalog logic: CatalogEntry dataclass, get_active_catalogs, _load_catalog_config, _fetch_single_catalog, _get_merged_extensions, updated search and get_extension_info, extended clear_cache |
src/specify_cli/__init__.py |
New catalog_app typer sub-app; new extension_catalogs, catalog_add, catalog_remove commands; _install_allowed enforcement in extension_add; catalog metadata display in extension_search and extension_info |
tests/test_extensions.py |
13 new tests covering CatalogEntry, get_active_catalogs, _load_catalog_config, merge conflict resolution, and _install_allowed propagation; updated test_search_all_extensions to use single-catalog config |
extensions/RFC-EXTENSION-SYSTEM.md |
Rewrote "Custom Catalogs" section to document the implemented catalog stack (was "FUTURE FEATURE") |
extensions/EXTENSION-USER-GUIDE.md |
Updated "Finding Extensions" and "Extension Catalogs" sections; new catalog management documentation |
extensions/EXTENSION-API-REFERENCE.md |
Added CatalogEntry API docs; updated ExtensionCatalog methods; added new CLI command docs |
Comments suppressed due to low confidence (1)
src/specify_cli/init.py:1830
- At line 1829,
catalog._load_catalog_config(config_path)is called afterget_active_catalogs()has already succeeded. However, ifSPECKIT_CATALOG_URLis set AND a.specify/extension-catalogs.ymlexists with an invalid URL (HTTP non-localhost), then:
get_active_catalogs()returns early using the env var (succeeds), so noValidationErroris raised there.- But the
_load_catalog_config(config_path)call on line 1829 then independently validates the URLs in the file, and will throw an unhandledValidationError.
This ValidationError is not caught in extension_catalogs(), causing an unhandled exception crash instead of a clean error message. The call on line 1829 should be wrapped in a try/except, or the intent of this second _load_catalog_config call should be to check only file existence (not URL validity).
config_path = project_root / ".specify" / "extension-catalogs.yml"
if config_path.exists() and catalog._load_catalog_config(config_path) is not None:
console.print(f"[dim]Config: {config_path.relative_to(project_root)}[/dim]")
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| config_path = project_root / ".specify" / "extension-catalogs.yml" | ||
| if config_path.exists() and catalog._load_catalog_config(config_path) is not None: | ||
| console.print(f"[dim]Config: {config_path.relative_to(project_root)}[/dim]") | ||
| elif os.environ.get("SPECKIT_CATALOG_URL"): | ||
| console.print("[dim]Catalog configured via SPECKIT_CATALOG_URL environment variable.[/dim]") | ||
| else: | ||
| console.print("[dim]Using built-in default catalog stack.[/dim]") | ||
| console.print( | ||
| "[dim]Add .specify/extension-catalogs.yml to customize.[/dim]" | ||
| ) |
There was a problem hiding this comment.
The config source detection in extension_catalogs (lines 1828–1836) doesn't account for the user-level config path ~/.specify/extension-catalogs.yml. When a user-level config is active (i.e., no project config and no SPECKIT_CATALOG_URL), the function falls through to the else branch and displays "Using built-in default catalog stack." — which is incorrect. The footer message should also check whether a user-level config is active, similar to how the project-level config is detected.
This issue also appears on line 1828 of the same file.
| Returns: | ||
| URL to fetch catalog from | ||
| Ordered list of CatalogEntry objects, or None if file doesn't exist | ||
| or contains no valid catalog entries. |
There was a problem hiding this comment.
The docstring for _load_catalog_config says it returns None when the file "contains no valid catalog entries", but when a ValidationError is raised inside the method (from _validate_catalog_url), it is not caught by the except (yaml.YAMLError, OSError) handler on line 1021. As a result, an invalid URL in the config file causes a ValidationError to propagate to callers (including get_active_catalogs), rather than returning None as the documentation implies.
This is an inconsistency between the documented behavior and actual behavior. The docstring should be updated to document the Raises: ValidationError case, or alternatively, ValidationError should be added to the exception handler if the intent is truly to return None on all errors.
| or contains no valid catalog entries. | |
| or contains no valid catalog entries. | |
| Raises: | |
| ValidationError: If any catalog entry has an invalid URL. |
|
|
||
| def test_search_all_extensions(self, temp_dir): | ||
| """Test searching all extensions without filters.""" | ||
| import yaml as yaml_module | ||
|
|
||
| project_dir = temp_dir / "project" | ||
| project_dir.mkdir() | ||
| (project_dir / ".specify").mkdir() | ||
|
|
||
| # Use a single-catalog config so community extensions don't interfere | ||
| config_path = project_dir / ".specify" / "extension-catalogs.yml" | ||
| with open(config_path, "w") as f: | ||
| yaml_module.dump( | ||
| { | ||
| "catalogs": [ | ||
| { | ||
| "name": "test-catalog", | ||
| "url": ExtensionCatalog.DEFAULT_CATALOG_URL, | ||
| "priority": 1, | ||
| "install_allowed": True, | ||
| } | ||
| ] | ||
| }, | ||
| f, | ||
| ) | ||
|
|
||
| catalog = ExtensionCatalog(project_dir) | ||
|
|
||
| # Create mock catalog |
There was a problem hiding this comment.
The test_search_all_extensions test was correctly updated to write a single-catalog config (to prevent the community catalog from being fetched). However, several other existing tests in TestExtensionCatalog that are not part of this diff (test_search_by_query, test_search_by_tag, test_search_verified_only, test_get_extension_info) don't write a catalog config and will now use the new default two-catalog stack. This causes them to attempt a network fetch of the community catalog during test runs. In an offline CI environment, this produces spurious stderr warnings. These tests should also be updated to use a single-catalog config (as done for test_search_all_extensions) to preserve test isolation.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/specify_cli/extensions.py:1021
- The
exceptclause on line 1021 only catchesyaml.YAMLErrorandOSError, butint(item.get("priority", idx + 1))on line 1016 can raiseValueErrorif thepriorityfield contains a non-integer value in the YAML (e.g.,priority: "not-a-number"). Similarly,bool(item.get("install_allowed", True))on line 1017 can raiseTypeErrorin edge cases. These exceptions would propagate as unhandled Python errors through_load_catalog_config()andget_active_catalogs(), resulting in a raw traceback for the user instead of a graceful fallback.
The except handler should also catch (ValueError, TypeError) to handle malformed YAML values gracefully, consistent with how YAML parsing errors are handled.
priority=int(item.get("priority", idx + 1)),
install_allowed=bool(item.get("install_allowed", True)),
))
entries.sort(key=lambda e: e.priority)
return entries if entries else None
except (yaml.YAMLError, OSError):
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| url=url, | ||
| name=str(item.get("name", f"catalog-{idx + 1}")), | ||
| priority=int(item.get("priority", idx + 1)), | ||
| install_allowed=bool(item.get("install_allowed", True)), |
There was a problem hiding this comment.
There's an inconsistency in the default value for install_allowed between _load_catalog_config (line 1017 in extensions.py) and the catalog_add CLI command (line 1845-1848 in __init__.py).
In _load_catalog_config, install_allowed defaults to True when the key is missing from YAML: bool(item.get("install_allowed", True)). But the catalog_add CLI command defaults to False for install_allowed.
This means that a user who manually edits the YAML to add a catalog without specifying install_allowed will get a catalog with install_allowed=True, but a user who uses the CLI catalog add command without specifying --install-allowed will get a catalog with install_allowed=False. This inconsistency can be confusing and lead to unexpected behavior depending on how the catalog is configured.
This issue also appears on line 1016 of the same file.
| install_allowed=bool(item.get("install_allowed", True)), | |
| install_allowed=bool(item.get("install_allowed", False)), |
ExtensionCatalogimplementationCatalogEntrydataclass and catalog stack logic toextensions.py__init__.py(catalogs, catalog add/remove)extension_add/extension_search/extension_infofprefix from strings with no placeholdersOriginal prompt
This section details on the original issue you should resolve
<issue_title>feat(extensions): support multiple active catalogs simultaneously</issue_title>
<issue_description>## Summary
The current extension catalog system supports only a single active catalog at a time — selectable via
SPECKIT_CATALOG_URLor defaulting to the built-incatalog.json. This creates a fundamental conflict:catalog.jsonis intentionally empty (organizations curate their own approved extensions), whilecatalog.community.jsonexists as a shared discovery resource, and organizations also need to point at their own private/internal catalogs. Users currently have no way to benefit from all three simultaneously.Problem Statement
The RFC describes a "Dual Catalog System" but the two catalogs serve different purposes and neither is composable with the other today:
catalog.jsoncatalog.community.jsonReal-world usage requires all three active at once: search the community catalog for discoverability, restrict installs to org-approved entries, and also pull from an internal catalog — with clear precedence rules between them.
Proposed Solution
Introduce a catalog stack — an ordered list of catalogs the CLI merges and searches across. Each catalog entry has a
url, an optionalname, apriority, and aninstall_allowedflag.Default built-in stack (no config required)
When no
.specify/extension-catalogs.ymlexists, the CLI uses a built-in default stack:catalog.json(org-curated,install_allowed: true, priority 1)catalog.community.json(community discovery,install_allowed: false, priority 2)This means
specify extension searchworks out of the box and surfaces community extensions, whilespecify extension addis still restricted to whatever is incatalog.json— preserving the existing curation/trust model.New config file:
.specify/extension-catalogs.yml(project-scoped)An equivalent user-level config lives at
~/.specify/extension-catalogs.ymlfor user-wide defaults. When a project-level config is present, it takes full control and the built-in defaults are not applied.Resolution order
When a user runs
specify extension searchorspecify extension add <name>, the CLI:id)install_allowed: false— extensions from discovery-only catalogs are shown in search results but cannot be installed directlyCLI additions
Backward compatibility
catalog.community.jsonasinstall_allowed: false— no config needed to get community discoverabilitySPECKIT_CATALOG_URLenv var still works: treated as a singleinstall_allowed: truecatalog, replacing both defaults for full backward compat.specify/extension-catalogs.ymloverrides all defaults entirelyAcceptance Criteria
specify extension catalogslists all active catalogs with name, URL, priority, andinstall_allowedcatalog.community.jsonis included in the default stack asinstall_allowed: falseat priority 2specify extension searchaggregates results across all active catalogs, annotating each result with its source catalogspecify extension addrespectsinstall_allowed: falseand rejects installs from discovery-only catalogs with a clear message: "'linear' is available in the community catalog but not in your approved catalog. Add it to.specify/extension-catalogs.ymlwithinstall_allowed: trueto enable installation.".specify/extension-catalogs.ymland `~/.specify/extension-cata...🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.